feat: ios fabric transform origin #38559

intergalacticspacehighway · 2023-07-23T11:56:56Z

Summary:

This PR adds transform-origin support for iOS fabric. This PR also incorporates feedback/changes suggested by @javache in the original PR.

Changelog:

[IOS] [ADDED] - Fabric Transform origin

Test Plan:

Run iOS RNTester app in old architecture and test transform-origin example in TransformExample.js.

github-actions · 2023-07-23T11:57:56Z

	Warnings
⚠️	packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js#L11 - packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js line 11 – Requires should be sorted alphabetically (lint/sort-imports)

Generated by 🚫 dangerJS against a30ed3d

analysis-bot · 2023-07-23T12:21:00Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,962,403	+2,327
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	9,556,421	+2,331
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: a0a544f
Branch: main

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

intergalacticspacehighway · 2023-07-27T07:11:06Z

Getting this error on test_android CI 🤔

C/C++: /root/react-native/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h:19:10: fatal error: 'yoga/YGValue.h' file not found

facebook-github-bot · 2023-08-21T18:31:19Z

@rozele has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

javache

Thanks for splitting these up into multiple PR's!

packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp

packages/react-native/Libraries/StyleSheet/private/_TransformStyle.js

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

javache · 2023-08-24T12:00:54Z

packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js

@@ -727,6 +727,7 @@ export type ____ViewStyle_InternalCore = $ReadOnly<{
  opacity?: AnimatableNumericValue,
  elevation?: number,
  pointerEvents?: 'auto' | 'none' | 'box-none' | 'box-only',
+  transformOrigin?: string,


Why is this required? transformOrigin should be included via ____TransformStyle_Internal already.

My bad. Updated!

javache · 2023-08-24T12:01:37Z

packages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp

+namespace {
+
+std::array<float, 3> getTranslateForTransformOrigin(float viewWidth, float viewHeight, facebook::react::TransformOrigin transformOrigin)  {
+  float viewCenterX = viewWidth / 2;
+  float viewCenterY = viewHeight / 2;
+
+  std::array<float, 3> origin = {viewCenterX, viewCenterY, transformOrigin.z};
+
+  for (size_t i = 0; i < 2; ++i) {
+    auto& currentOrigin = transformOrigin.xy[i];
+    if (currentOrigin.unit == YGUnitPoint) {
+      origin[i] = currentOrigin.value;
+    } else if (currentOrigin.unit == YGUnitPercent) {
+        origin[i] = ((i == 0) ? viewWidth : viewHeight) * currentOrigin.value / 100.0f;
+      }
+    }
+
+
+  float newTranslateX = -viewCenterX + origin[0];
+  float newTranslateY = -viewCenterY + origin[1];
+  float newTranslateZ = origin[2];
+
+  return std::array{newTranslateX, newTranslateY, newTranslateZ};
+}
+
+}


nit: move inside the namespace facebook::react below (and remove the extra facebook::react prefixes)

javache · 2023-08-24T12:03:18Z

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

+  TransformOrigin transformOrigin;
+
+   for (size_t i = 0; i < 2; i++) {
+     const auto& origin = origins[i];
+     if (origin.hasType<Float>()) {
+       transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);
+     } else if (origin.hasType<std::string>()) {
+       const auto stringValue = (std::string)origin;
+
+       if (stringValue.back() == '%') {
+         auto tryValue = folly::tryTo<float>(
+                                             std::string_view(stringValue).substr(0, stringValue.length() - 1));
+         if (tryValue.hasValue()) {
+           transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};
+         }
+       }
+     }
+   }
+
+    if (origins[2].hasType<Float>()) {
+      transformOrigin.z = (Float)origins[2];
+    }


You do need to check now that the indices you're accessing are not exceeding the vector bounds.

Suggested change

TransformOrigin transformOrigin;

for (size_t i = 0; i < 2; i++) {

const auto& origin = origins[i];

if (origin.hasType<Float>()) {

transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);

} else if (origin.hasType<std::string>()) {

const auto stringValue = (std::string)origin;

if (stringValue.back() == '%') {

auto tryValue = folly::tryTo<float>(

std::string_view(stringValue).substr(0, stringValue.length() - 1));

if (tryValue.hasValue()) {

transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};

}

}

}

}

if (origins[2].hasType<Float>()) {

transformOrigin.z = (Float)origins[2];

}

TransformOrigin transformOrigin;

for (size_t i = 0; i < std::min(origins.size(), 2); i++) {

const auto& origin = origins[i];

if (origin.hasType<Float>()) {

transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);

} else if (origin.hasType<std::string>()) {

const auto stringValue = (std::string)origin;

if (stringValue.back() == '%') {

auto tryValue = folly::tryTo<float>(

std::string_view(stringValue).substr(0, stringValue.length() - 1));

if (tryValue.hasValue()) {

transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};

}

}

}

}

if (origin.size() >= 3 && origins[2].hasType<Float>()) {

transformOrigin.z = (Float)origins[2];

}

javache · 2023-08-24T12:04:27Z

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h

@@ -50,6 +51,23 @@ struct TransformOperation {
  Float z;
 };

+struct TransformOrigin {
+  std::array<YGValue, 2> xy;


Per @NickGerleman feedback on another PR, using YGValue is an undesired dependency. Let's introduce a custom struct for this in another PR.

Added a PR for struct - #39150. I have tested it replacing YGValue and it works, kept it simple. let me know!

std::array<YGValue, 2> xy; -> std::array<ValueUnit, 2> xy;

cc - @jacobp100 we can also use it here - #38626

Will try and get to it later this weekend

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

…ormorigin

jacobp100 · 2023-08-25T11:36:57Z

packages/react-native/ReactCommon/react/renderer/graphics/ValueUnit.h

+
+namespace facebook::react {
+
+enum class UnitType {


Why do we need undefined?

To represent initial/undefined value? Also useful to check the value is set. Check isSet in TransformOrigin struct.

facebook-github-bot · 2023-08-29T10:06:39Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: This PR adds transform-origin support for iOS fabric. This PR also incorporates feedback/changes suggested by javache in the original [PR.](facebook#37606) ## Changelog: [IOS] [ADDED] - Fabric Transform origin <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#38559 Test Plan: Run iOS RNTester app in old architecture and test transform-origin example in `TransformExample.js`. Differential Revision: D48528363 Pulled By: javache fbshipit-source-id: 21cfe06db750c8cf55d43039f0189089d29fca6f

Summary: Adds transform-origin for old arch iOS See also intergalacticspacehighway's work for iOS Fabric and Android:- - facebook#38559 - facebook#38558 ## Changelog: [IOS] [ADDED] - Added support for transform-origin on old arch Pull Request resolved: facebook#38626 Test Plan: See RN tester Differential Revision: D48528353 Pulled By: javache fbshipit-source-id: 0189f374c9556b6593b3d72d7503b9cf166378c2

javache · 2023-09-01T13:33:39Z

This is now ready to ship internally so should be pushed soon. Please also submit a PR to https://github.com/facebook/react-native-website to add documentation for this.

jacobp100 · 2023-09-01T14:54:13Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

Summary: Adds transform-origin for old arch iOS See also intergalacticspacehighway's work for iOS Fabric and Android:- - #38559 - #38558 ## Changelog: [IOS] [ADDED] - Added support for transform-origin on old arch Pull Request resolved: #38626 Test Plan: See RN tester Reviewed By: NickGerleman Differential Revision: D48528353 Pulled By: javache fbshipit-source-id: 09592411e26484d81a999a7e601eff751ca7ae0b

facebook-github-bot · 2023-09-01T20:08:31Z

@javache merged this pull request in 177ef21.

javache · 2023-09-05T10:51:01Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

Doesn't 5f40f08 handle the old arch? What are we missing still?

jacobp100 · 2023-09-05T10:53:43Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

Doesn't 5f40f08 handle the old arch? What are we missing still?

I think the plan was to remove the dependency on Yoga in this - 5f40f08#diff-6654f56131a3cc2c701d1ca11fbdd2c4c41854136153eaceba4a103413e81809R11-R15

So make our own struct that stores the float value, and whether it was a percent or pixel value

javache · 2023-09-05T13:56:34Z

I think the plan was to remove the dependency on Yoga in this - 5f40f08#diff-6654f56131a3cc2c701d1ca11fbdd2c4c41854136153eaceba4a103413e81809R11-R15

So make our own struct that stores the float value, and whether it was a percent or pixel value

I think we're ok as-is, as we've been able to cleanup the dependency in the new arch version.

feat: transform origin ios fabric

4b13fb1

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 23, 2023

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 23, 2023

intergalacticspacehighway mentioned this pull request Jul 23, 2023

feat: transform-origin #37606

Closed

4 tasks

jacobp100 reviewed Jul 25, 2023

View reviewed changes

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm Show resolved Hide resolved

intergalacticspacehighway added 2 commits July 25, 2023 23:51

move transform in invalidateLayer

26b8430

fix: lint errors

e65fbf3

jacobp100 mentioned this pull request Jul 25, 2023

Implement transform-origin for old arch iOS #38626

Closed

intergalacticspacehighway added 2 commits July 26, 2023 01:34

revert needs invalidate layer

fbaf6c0

transform origin struct

35badaf

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h Show resolved Hide resolved

intergalacticspacehighway added 2 commits July 26, 2023 17:27

nit: pr suggestions

302a9aa

revert non fabric changes

be6b4d5

intergalacticspacehighway changed the title ~~feat(ios(fabric)): transform origin~~ feat: ios fabric transform origin Jul 27, 2023

intergalacticspacehighway added 2 commits July 27, 2023 11:26

fix: tests

ffa6afc

nit: header fix

ab174da

intergalacticspacehighway added 2 commits August 7, 2023 11:25

merge main

a9632d6

merge main

eefec92

javache approved these changes Aug 23, 2023

View reviewed changes

intergalacticspacehighway added 2 commits August 23, 2023 23:01

fix: PR feedback

40ed8db

fix: PR feedback

8902301

javache mentioned this pull request Aug 24, 2023

feat: android transform origin #38558

Closed

javache reviewed Aug 24, 2023

View reviewed changes

intergalacticspacehighway added 5 commits August 24, 2023 17:51

PR feedback

0e91714

remove redundant type

a74fdf8

fix min error

854710a

fix min error

9a3c230

add value unit struct

55ad2c9

intergalacticspacehighway mentioned this pull request Aug 24, 2023

feat: add ValueUnit struct #39150

Closed

intergalacticspacehighway and others added 3 commits August 25, 2023 03:24

Apply suggestions from code review

696621e

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Merge branch 'feat/add-value-unit-struct' into feat/ios-fabric-transf…

0f27a0c

…ormorigin

use ValueUnit struct

a30ed3d

jacobp100 reviewed Aug 25, 2023

View reviewed changes

intergalacticspacehighway mentioned this pull request Sep 1, 2023

docs: transform origin facebook/react-native-website#3842

Merged

facebook-github-bot closed this in 177ef21 Sep 1, 2023

facebook-github-bot added the Merged This PR has been merged. label Sep 1, 2023

intergalacticspacehighway deleted the feat/ios-fabric-transformorigin branch August 9, 2024 22:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ios fabric transform origin #38559

feat: ios fabric transform origin #38559

intergalacticspacehighway commented Jul 23, 2023 •

edited

Loading

github-actions bot commented Jul 23, 2023 •

edited

Loading

analysis-bot commented Jul 23, 2023 •

edited

Loading

intergalacticspacehighway commented Jul 27, 2023

facebook-github-bot commented Aug 21, 2023

javache left a comment

javache Aug 24, 2023

intergalacticspacehighway Aug 24, 2023

javache Aug 24, 2023

intergalacticspacehighway Aug 24, 2023

javache Aug 24, 2023

intergalacticspacehighway Aug 24, 2023

javache Aug 24, 2023

intergalacticspacehighway Aug 24, 2023

jacobp100 Aug 25, 2023

jacobp100 Aug 25, 2023

intergalacticspacehighway Aug 25, 2023

facebook-github-bot commented Aug 29, 2023

javache commented Sep 1, 2023

jacobp100 commented Sep 1, 2023

facebook-github-bot commented Sep 1, 2023

javache commented Sep 5, 2023

jacobp100 commented Sep 5, 2023

javache commented Sep 5, 2023

feat: ios fabric transform origin #38559

feat: ios fabric transform origin #38559

Conversation

intergalacticspacehighway commented Jul 23, 2023 • edited Loading

Summary:

Changelog:

Test Plan:

github-actions bot commented Jul 23, 2023 • edited Loading

analysis-bot commented Jul 23, 2023 • edited Loading

intergalacticspacehighway commented Jul 27, 2023

facebook-github-bot commented Aug 21, 2023

javache left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Aug 29, 2023

javache commented Sep 1, 2023

jacobp100 commented Sep 1, 2023

facebook-github-bot commented Sep 1, 2023

javache commented Sep 5, 2023

jacobp100 commented Sep 5, 2023

javache commented Sep 5, 2023

intergalacticspacehighway commented Jul 23, 2023 •

edited

Loading

github-actions bot commented Jul 23, 2023 •

edited

Loading

analysis-bot commented Jul 23, 2023 •

edited

Loading